-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
work-in-progress: redis integration #1
base: master
Are you sure you want to change the base?
Conversation
On reflection, I think the .get & .set parameters should be more consistent with each other: ie: request.user.session.set("foo",999)
foo = request.user.session.get("foo") |
@stock90975: Great job! This will need some more thought and work, but I think we are heading in the right direction here :) I already have some questions / feedback:
|
@fscherf Thanks!!!
Glad you agree, I should have elaborated. If we pick some Redis settings, then
Hmm ... I need to study this to understand how lona works 😀
Redis (and redis-py) is "limited" in what datatypes can be stored, eg: # start first Redis with:
# docker run -p 6379:6379 -it redis:latest
import redis
r = redis.Redis(host='localhost', port=6379, db=0)
r.set('foo', True)
# redis.exceptions.DataError: Invalid input of type: 'bool'. Convert to a bytes, string, int or float first. 🙄 Even most basic Python datatypes are not accepted "as-is", let alone dicts or list or other wierd combos. My examples demonstrate that lona-redis could cope with these easily. Anything pickle-able will work. Hopefully users will only want to store pickle-able values! Using pickle IS a sledgehammer approach, but I coudn't think of a better way. I'm definitely open to suggestions, as this is a possible speed penalty, that might detract from using Redis in the first place. Redis has some super-sophisticated powers! (https://redis.readthedocs.io/en/latest/commands.html#) For this reason also, I think lona-redis should expose the r. object to the user so they can easily get to these commands if they really want to. Hence, the dual approach: use lona-redis the "easy" way or the hardcore way... |
I my mymiddlewares.py: def handle_request(self, data):
view = data.view
# view.is_daemon is always False even though in the view def handle_request(self, request):
self.is_daemon = True How do I check if a view is a daemon-view from middlewares? |
I think we then should go with your first approach and just have On the pickle question: Ok, makes sense to me for now. To be honest, I never used redis-py before. We will see if this creates performance problems down the line, but I think its ok for now. On the middleware/daemon question: |
I've used a Redis in Flask - when trying to use it directly, it got quite ugly. I believe this implementation will give the user better access to the full functionality, but without sacrificing ease of use.
Just trying to understand how views work to address #2:
TBH I'm a bit lost as to how to do this because I don't get how views would affect the use of lona-redis, so I'm stuck as to how to proceed here. I will clean up my code for the Redis part of the lona-redis middleware and write up usage docs soon. |
Could it be that we mean different things when we say |
Hmm, I was thinking more as cookies, using Redis as the key-value store. Anyway, please have a look at the latest code & docs and let me know what you think. I was hoping for just a basic cookie set() & get() that lona could do out of the box. (Maybe it does but I just don't know how...) |
@stock90975: But every user should have its own key-value store right? Lona identifies each user by a session cookie, which gets randomly generated on the first connect (no username-password pair). Should we use this here too, or should we implement some password infrastructure like in lona-django? The code looks ok, but the commits are pretty messy |
Sorry about the commits - I'm learning git. Should I be commiting each file individually or multiple files as a commit?
Yes I'm using request.user.session_key and redis_key() Since request.user.session_key is unique, this should distinguish one user's key from another user's key right? (I think...) |
No problem! I am sorry, If I was a bit harsh. Every commit should do only one thing, like fix one bug, or add one feature, regardless of how many files are changed. It's often hard to tell what "one thing" is. For example, I am currently working on channels for Lona. That changes many files at once, but these changes only work in conjunction, so it makes no sense to split them into multiple commits. In this case I am completely fine with one big commit with a commit message like "initial code commit", because every "change" is an "add" currently.
Ok great! So Lona can tell apart two different user then. To do so we have provide Lona a custom User model which implements Basically Lona does this: def new_connection_comes_in(connection, view):
if connection.user == view.user:
view.add_connection(connection)
else:
raise ForbiddenError |
Ok thanks for understanding! I really am a noob ... Ok, so what is the next step? BTW Further to using Redis as the backend, do you think it would it be useful / good / attractive for those new to lona to see that it can deal with a session cookies out-of-the-box? Specifically I'm thinking of having a version that does not need Redis, for really simple or demo purposes. eg. the backend for the key-value pairs is just a Python dict. eg. this case would be triggered if settings.REDIS_SETTINGS was missing. |
Lona is all about being as batteries-included with useful defaults as possible so I would want lona-redis to be able to handle sessions, but it should be opt-in, so you will have to switch it on. What do you mean by "a version that does not need redis"? Isn't this library all about integrating redis? If you want a simple key-value store, you already can use |
Oh I see, I didn't think about using server.state! |
Yes it is global, but you can do something like this: per_user_data = server.state.get(request.session.session_key, {}) |
Lots of updates:
README.md
example_redis.py
lona_redis/middlewares.py
tests
Everything is open for comment / changes
Thanks!